Skip to content

[TT-16980] Fix pgx v5 breaking change when postgres.prefer_simple_protocol is set to true#969

Merged
buraksezer merged 1 commit intoTT-16932from
fix/TT-16980/pgx-v5-breaking-change
Apr 17, 2026
Merged

[TT-16980] Fix pgx v5 breaking change when postgres.prefer_simple_protocol is set to true#969
buraksezer merged 1 commit intoTT-16932from
fix/TT-16980/pgx-v5-breaking-change

Conversation

@buraksezer
Copy link
Copy Markdown
Contributor

PR for https://tyktech.atlassian.net/browse/TT-16980

The proposed solution(BeforeCreate + SetColumn) didn't work because GORM's ConvertToCreateValues reads field values via field.ValueOf(), which uses reflection to return fieldValue.Interface(). Since the struct field type is time.Month, the value always reaches pgx as time.Month regardless of what SetColumn wrote. Go's reflection preserves the named type.

Another possible solution might be to change the data type of Month field in AnalyticsRecord. The current type is time.Month, if we create a new new type that implements driver.Valuer but it is a potential breaking change and requires update across all Tyk components other potentials dependents.

Implemented solution: In pumps/sql.go, I replaced the postgres case in Dialect() to build the *sql.DB ourselves via stdlib.OpenDB with an OptionAfterConnect callback.

On each new connection, we prepend a custom TryWrapEncodePlanFunc to the encode plan chain that intercepts time.Month and converts it to int before pgx's fmt.Stringer wrapper can fire. I also replicate the timezone extraction logic from gorm.io/driver/postgres to avoid behavioral regression, and pass the pre-built *sql.DB to GORM via postgres.Config{Conn: sqlDB}.

I have also added a test named TestSQLWriteData_PreferSimpleProtocol_Month, before the fix that test was failing.

Skip is also removed from TestPreferSimpleProtocol_Postgres.

How to run tests:

TYK_TEST_POSTGRES="host=127.0.0.1 port=5432 user=postgres password=postgres dbname=postgres_test sslmode=disable" go test ./pumps/ -run TestSQLWriteData_PreferSimpleProtocol_Month -v
TYK_TEST_POSTGRES="host=127.0.0.1 port=5432 user=postgres password=postgres dbname=postgres_test sslmode=disable" go test ./pumps/ -run TestPreferSimpleProtocol_Postgres -v

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Apr 17, 2026

This PR addresses a breaking change introduced by the pgx v5 driver when using PostgreSQL with prefer_simple_protocol set to true. The issue caused time.Month fields to be encoded as strings (e.g., "May") instead of integers, leading to database errors when inserting analytics records.

The fix intercepts the database connection process for the PostgreSQL dialect. Instead of letting the GORM driver create the connection, this change manually constructs a *sql.DB instance using pgx. It injects an AfterConnect callback that prepends a custom encoding plan to the pgx type map. This new plan ensures that time.Month values are converted to integers before the default string-based encoding can be applied.

To validate the solution, a new test (TestSQLWriteData_PreferSimpleProtocol_Month) has been added, and a previously skipped test (TestPreferSimpleProtocol_Postgres) has been re-enabled.

Files Changed Analysis

  • go.mod: Dependency management file updated, reflecting a reordering of the pgx/v5 entry, likely due to go mod tidy.
  • pumps/sql.go: Contains the core logic. The Dialect function for PostgreSQL is modified to manually configure the pgx connection, register a custom type encoder for time.Month, and then pass the pre-configured connection to GORM.
  • pumps/sql_pgxv5_test.go: A new test is added to specifically verify that time.Month is correctly persisted as an integer with prefer_simple_protocol=true. An existing test that was skipped due to this bug has been re-enabled.

Architecture & Impact Assessment

  • Accomplishment: Fixes a critical bug preventing the use of the SQL pump with PostgreSQL when the prefer_simple_protocol option is enabled, ensuring analytics data is saved correctly.
  • Key Technical Changes: The main change is moving from a GORM-managed DB connection to a manually configured *sql.DB instance for PostgreSQL. This allows for low-level customization of the pgx driver's data type encoding behavior via an AfterConnect hook.
  • Affected Components: This change affects Tyk Pump instances configured to use the SQL pump with a PostgreSQL backend, specifically those that enable the prefer_simple_protocol setting.

Connection and Encoding Flow

sequenceDiagram
    participant Pump as "Tyk Pump (sql.go)"
    participant GORM as "gorm.io/gorm"
    participant PGX as "github.com/jackc/pgx/v5"
    participant PG as "PostgreSQL DB"

    Pump->>Pump: Dialect("postgres") called
    Pump->>PGX: pgx.ParseConfig(dsn)
    Pump->>PGX: stdlib.OpenDB(pgxConfig, OptionAfterConnect)
    Note right of Pump: AfterConnect callback is configured
    PGX->>PG: Establishes new connection
    PG-->>PGX: Connection successful
    PGX->>Pump: Executes AfterConnect callback
    Pump->>PGX: Prepend custom encoder for time.Month
    PGX-->>Pump: Returns *sql.DB
    Pump->>GORM: postgres.New(Config{Conn: sqlDB})
    Note over Pump, PG: Later, when writing data...
    GORM->>PGX: Executes query with time.Month
    PGX->>PGX: Custom encoder converts time.Month to int
    PGX->>PG: Sends SQL with integer value for month
Loading

Scope Discovery & Context Expansion

  • Scope: The change is well-contained within the SQL pump's PostgreSQL dialect implementation. It directly addresses the interaction between GORM, pgx v5, and the time.Month data type.
  • Context: The analytics.AnalyticsRecord struct, defined in analytics/analytics.go, contains the Month time.Month field that was causing the issue. The SQLPump.WriteData function in pumps/sql.go uses gorm.DB.Create to persist these records, which triggers the encoding logic that this PR corrects. The fix at the driver-level is robust as it covers all writes of this data type through the pump without requiring changes to the data model or application logic.
Metadata
  • Review Effort: 3 / 5
  • Primary Label: bug

Powered by Visor from Probelabs

Last updated: 2026-04-17T08:33:29.697Z | Triggered by: pr_opened | Commit: 116d4c2

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor

probelabs Bot commented Apr 17, 2026

✅ Security Check Passed

No security issues found – changes LGTM.

✅ Architecture Check Passed

No architecture issues found – changes LGTM.

✅ Performance Check Passed

No performance issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2026-04-17T08:31:44.452Z | Triggered by: pr_opened | Commit: 116d4c2

💡 TIP: You can chat with Visor using /visor ask <your question>

@sonarqubecloud
Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

@buraksezer buraksezer merged commit 7db02d4 into TT-16932 Apr 17, 2026
73 of 95 checks passed
@buraksezer buraksezer deleted the fix/TT-16980/pgx-v5-breaking-change branch April 17, 2026 13:55
buraksezer added a commit that referenced this pull request Apr 17, 2026
buger added a commit that referenced this pull request Apr 17, 2026
* fix CVE-2026-32286

* update to pgx v5.9.1

* test gorm fork update

* fix CI

* tests for gorm update

* fix mysql test

* more tests

* skip TestPreferSimpleProtocol

* [TT-16980] Fix pgx v5 breaking change when postgres.prefer_simple_protocol is set to true (#969)

---------

Co-authored-by: Burak Sezer <burak.sezer.developer@gmail.com>
Co-authored-by: Leonid Bugaev <leonsbox@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants